feat: add client-side filesize check to file component #21
Conversation
RealZone22
left a comment
RealZone22
left a comment
There was a problem hiding this comment.
Thanks for your patience and your contribution.
I appreciate the effort you put into this. Before we can move forward, I have a few requests to keep the code consistent with the project’s standards.
If you need any help or clarification, feel free to ask.
Looking forward to your updates.
| const file = event.target.files[0]; | ||
| const limit = {{ $maxSize }} * 1024; // KB'ı Byte'a çeviriyoruz | ||
| if (file && file.size > limit) { | ||
| alert('File is too large! Maximum size allowed is {{ $maxSize }}KB.'); |
There was a problem hiding this comment.
Please avoid using alert() for validation errors.
We already render validation messages below the input with the text-danger class (line 48–49).
Also, please do not hardcode the error message in English.
The project uses Laravel translations, so the message should come from the language files.
I will add the German translation on my side.
| 'showRequired' => true, | ||
| 'showValidation' => true, | ||
| 'tooltip' => null, | ||
| 'maxSize' => 2048, // KB cinsinden varsayılan limit (2MB) |
There was a problem hiding this comment.
I suggest changing maxSize to null by default.
A default limit of 2048 KB enforces behavior that may not be desired for all usages.
The inline comment can be removed, the intent of maxSize is already clear.
| uuid: Math.random().toString(20).substring(2, 20), | ||
| checkFileSize(event) { | ||
| const file = event.target.files[0]; | ||
| const limit = {{ $maxSize }} * 1024; // KB'ı Byte'a çeviriyoruz |
There was a problem hiding this comment.
The inline comment can be removed.
The conversion from KB to bytes is self explanatory and does not need a comment.
| const limit = {{ $maxSize }} * 1024; // KB'ı Byte'a çeviriyoruz | ||
| if (file && file.size > limit) { | ||
| alert('File is too large! Maximum size allowed is {{ $maxSize }}KB.'); | ||
| event.target.value = ''; // Seçimi temizle |
There was a problem hiding this comment.
Remove this comment.
Clearing the file input value is clear without explanation.
|
Hi @RealZone22, thank you so much for the detailed feedback and your kind words. I've just pushed the updates to bring the code in line with the project's standards. |
RealZone22
left a comment
RealZone22
left a comment
There was a problem hiding this comment.
Thanks for the updates. I still have a few more changes.
Good to hear you’re learning German.
By the way, you’re the first contributor to one of my projects, so this is my first time reviewing code.
| if (file && maxSize) { | ||
| const limit = maxSize * 1024; | ||
| if (file.size > limit) { | ||
| this.errorMessage = '{{ __("validation.size.numeric", ["attribute" => "file", "max" => $maxSize]) }}'; |
There was a problem hiding this comment.
This translations does not exists in Laravel (See: https://github.com/laravel/framework/blob/12.x/src/Illuminate/Translation/lang/en/validation.php#L101)
You can use this one instead:
this.errorMessage = '{{ __('validation.max.file', ['max' => $maxSize]) }}';| </p> | ||
| @endif | ||
|
|
||
| <div x-show="errorMessage" class="text-danger text-sm" x-text="errorMessage"></div> |
There was a problem hiding this comment.
The original div with x-show="errorMessage" is outside the x-data scope. That prevents AlpineJS error messages from showing. The easiest fix is to move this code to the end of the x-data div (after the input, Line 45).
Here is an approach that ensures only 1 validation message is shown at a time:
@if($showValidation)
<div class="text-danger text-sm">
<span x-show="errorMessage" x-text="errorMessage" x-cloak></span>
@if($attributes->whereStartsWith('wire:model')->first() && $errors->has($attributes->whereStartsWith('wire:model')->first()))
<span x-show="!errorMessage" x-cloak>{{ $errors->first($attributes->whereStartsWith('wire:model')->first()) }}</span>
@endif
</div>
@endifWe can then remove the old validation (Line: 55-58).
|
Thanks for the heads up! I've updated the translation key to validation.max.file. Also, I've implemented the conditional display logic you suggested to prevent multiple error messages. I'm honored to be the first contributor to your project! Your first review was very clear and helpful, thank you for the guidance. Looking forward to your thoughts. |
Added missing `</div>` tag
RealZone22
left a comment
RealZone22
left a comment
There was a problem hiding this comment.
Thanks again for the changes. I added the missing div closing tag.
Also, I wish you lots of success and good luck with your plans to work in Germany!
|
Thank you so much, @RealZone22! I'm very happy to have made my first contribution to PenguBlade. Thanks for the fix on the closing tag and for your kind wishes about Germany. It was a pleasure working with you. See you in future contributions! 😊 |
2 participants
I've implemented a client-side file size check using Alpine.js. This ensures that users cannot upload files larger than the specified maxSize. If a file is too large, it alerts the user and clears the input.
Fixes #20